docs(server): design the SubjectService token principal (RIG-2863) - #868
Open
rigel-mintaka wants to merge 8 commits into
Open
docs(server): design the SubjectService token principal (RIG-2863)#868rigel-mintaka wants to merge 8 commits into
rigel-mintaka wants to merge 8 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-server-rig-2863-subj.compass-eng-docs.pages.dev Deployed from Changed pages: |
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-3 review returned 0 high / 4 medium / 3 low. All six round-2 fixes held; the blockers were three new grounded defects in round-2's reconcile-execution- context paragraph (the WithSystemRole ruling itself is correct and stands) plus one ledger-recording judgment call Matt ruled on. - M1 (medium): the paragraph justified WithSystemRole by claiming FORCE RLS binds even the owner at boot. False — the owner connection is a superuser and a superuser bypasses even FORCE (0001_init.sql:848-851; rls_pgtest_test.go: 282-284). Restated on the true, superuser-independent reason: the reconcile takes the compass_app path, a non-owner non-BYPASSRLS role confined to the bootstrap tenant, so a row under another tenant is silently skipped without WithSystemRole. - M2 (medium): the prescribed two-tenant same-name test is unconstructible — `secrets.name` is a global PRIMARY KEY (0001_init.sql:396), so at most one row per configured name exists deployment-wide. Reframed the motivation and the test to the real case: a row declared under a NON-bootstrap tenant. - M3 (medium): corrected the "fifth entrypoint" miscount — the shipped tree arms WithSystemRole at THREE call sites (consumer.go:316, hub.go:753/814), and hub.go:814 (forge-notification-ack) is already a BYPASSRLS site DL-315's four names omit. Enumeration restated against the shipped set. - L1/L2/L3 (low): cite the arming code (armQueue/beginTenantTx) not just the WithSystemRole entrypoint; quote DL-315 verbatim; move the reconcile test into T0's Tests enumeration. - M4 -> OQ-5 (Matt-ruled: a new ledger row): the DL-315 BYPASSRLS allow-list widening lands as DL-326 (Refines DL-315, which stays Active), restating the true shipped entrypoint set incl. the previously-unledgered forge-notification- ack arm. Discoverability at the surface where the BYPASSRLS-audit question gets asked, over prose buried in an at-rest-encryption record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked. Renumbered this addendum's ledger rows to the next free sequential ids — the `gateway_credentials` encryption decision is DL-325 (was DL-324), the BYPASSRLS allow-list widening stays DL-326, and the sibling SubjectService PR (#868) takes DL-327 — leaving main's DL-324 untouched. Additive review-fix commit atop the round-2 fix. Ledger-impact: DL-325 (encryption, renumbered from DL-324), DL-326 (new, Refines DL-315) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…863) Review of #868 returned 0 high / 1 medium / 2 low. Citations (16/16) and the SubjectService ledger row were a clean pass; the blocker was executor-contract coherence. - F1 (medium): the Plan framed all five tasks as one "PR2 executor contract", but T4 (the service-door mount) targets a Server surface that does not exist in the tree yet, and no corpus task owns minting a SubjectService token — so the class as written ships unreachable. Reframed the Plan into two scopes: the enum half (T1/T2/T3/T5) lands now as PR2 (the urgent half — enum number + CHECK are painful to change once token rows exist); the door half (T4) lands WITH the RIG-2863 (RIG-1715 T2) service surface it mounts on, ordered after it. Added OQ-3 naming where token issuance lives (the RIG-2863 T4 slice, an IssueServiceToken mirroring MintRunnerToken) so an executor never improvises a mint path on a security-critical door. - F2 (low): citation span `lines 348-350` -> `348-351` (the quoted "disable)" falls on 351). - F3 (low, pre-existing): noted that `runnerhub.RunnerTokenRegistered` is the one token-store consumer outside the kind-gated door contract — flagged for the T4 executor to kind-check or document, out of scope for this record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked, and the sibling encryption PR (#859) took DL-325/DL-326. Renumbered this record's SubjectService row to the next free id, DL-327 (was DL-325), leaving main's DL-324 untouched. Additive review-fix commit. Ledger-impact: DL-327 (SubjectService, renumbered from DL-325) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-subject-service-design
branch
from
September 4, 2026 19:28
fc9b9ec to
abca4b8
Compare
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-3238) ReviewDesign869 returned 1 high / 2 medium / 4 low on PR #869. Grounding fidelity was clean (all code + cross-record citations verify at source); these are ledger-collision and plan-completeness fixes. All folded; the two mediums each also surface a load-bearing fork to Matt as a new OQ rather than being decided here. - HIGH (ledger collision): DL-326's Decision cell cited DL-325, an unlanded number double-claimed by #804 (RIG-3070) and #868 (RIG-2863). Since #868 is review-ready it may take DL-325 for an unrelated decision, and the cell is immutable after append. Re-cite the runner trust-model split by name+issue (RIG-3070 / PR #804), not the contested number, and add a Global Constraint ID-allocation + freeze-order note (mirroring the sibling embedded-revival record's GC6): re-grep main's ledger tail before landing, and the record does not depend on #804 landing first. - MEDIUM (T-4 premise gap): removing podman-machine removes the Linux VM the runner runs IN on macOS today (compass-local-dev:194-205 ruled the runner runs INSIDE the VM), so apple-container forces the runner to run natively on darwin. Add a T-1(f) spike leg, an Approach reason-2 caveat, a T-4 scope note, and OQ-12 (load-bearing Matt fork: does a NO re-open adoption?). - MEDIUM (T-2 podman surface): the stack has a second hard-coded podman shell (postgres_container.go:246-259 + collector_container.go:62) and ImageEnsurer is podman-hardwired at its production constructor (image.go:44-45). DL-260 keeps postgres a rootless-podman container, so a macOS host still needs podman for the stack. Widen T-2's off-interface enumeration, concede the scoped "no machine" win in Approach reason 2, and add OQ-13 (Matt fork: postgres to apple-container, or --database-external as the macOS default?). - LOW x4: nine-verbs wording (Resize is the ninth, not a tenth); image.go ImageExists citation span (:713, outside the cited :681-684); the OQ-1..OQ-4 gate carve-out for OQ-5's judgment call; DL-326 record link gains the §Approach anchor for the one-hop convention. Checks: markdownlint 0 errors; design-ledger-gate OK (291 rows, 122 headers). Status stays Draft — freezes Active on merge. Refs RIG-3238 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…-3238) ReviewDesign869 returned 1 high / 2 medium / 4 low on PR #869. Grounding fidelity was clean (all code + cross-record citations verify at source); these are ledger-collision and plan-completeness fixes. All folded; the two mediums each also surface a load-bearing fork to Matt as a new OQ rather than being decided here. - HIGH (ledger collision): DL-326's Decision cell cited DL-325, an unlanded number double-claimed by #804 (RIG-3070) and #868 (RIG-2863). Since #868 is review-ready it may take DL-325 for an unrelated decision, and the cell is immutable after append. Re-cite the runner trust-model split by name+issue (RIG-3070 / PR #804), not the contested number, and add a Global Constraint ID-allocation + freeze-order note (mirroring the sibling embedded-revival record's GC6): re-grep main's ledger tail before landing, and the record does not depend on #804 landing first. - MEDIUM (T-4 premise gap): removing podman-machine removes the Linux VM the runner runs IN on macOS today (compass-local-dev:194-205 ruled the runner runs INSIDE the VM), so apple-container forces the runner to run natively on darwin. Add a T-1(f) spike leg, an Approach reason-2 caveat, a T-4 scope note, and OQ-12 (load-bearing Matt fork: does a NO re-open adoption?). - MEDIUM (T-2 podman surface): the stack has a second hard-coded podman shell (postgres_container.go:246-259 + collector_container.go:62) and ImageEnsurer is podman-hardwired at its production constructor (image.go:44-45). DL-260 keeps postgres a rootless-podman container, so a macOS host still needs podman for the stack. Widen T-2's off-interface enumeration, concede the scoped "no machine" win in Approach reason 2, and add OQ-13 (Matt fork: postgres to apple-container, or --database-external as the macOS default?). - LOW x4: nine-verbs wording (Resize is the ninth, not a tenth); image.go ImageExists citation span (:713, outside the cited :681-684); the OQ-1..OQ-4 gate carve-out for OQ-5's judgment call; DL-326 record link gains the §Approach anchor for the one-hop convention. Checks: markdownlint 0 errors; design-ledger-gate OK (291 rows, 122 headers). Status stays Draft — freezes Active on merge. Refs RIG-3238 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…sign (RIG-3238) ReviewDesign869r2 verified all 7 round-1 findings CLOSED but caught two mediums the round-1 fold's own new prose introduced. Both are mechanical text corrections with no design consequence. - MEDIUM (guard-bullet facts): the ID-allocation/freeze-order Global Constraint bullet had wrong PR->DL attributions on every clause. Ground truth (verified via gh pr diff + main's ledger): main's tail ends at DL-324 (landed by #870, merged); DL-325 is claimed by #804 (RIG-3070, draft) AND #859 (RIG-2863); DL-326 -- this record's own number -- is claimed by #859, which is review-ready (NOT draft). The bullet missed the real live collision (#859 on DL-326) and named #868 (which actually claims DL-327). Rewrote the factual clauses to match; kept the two correct load-bearing clauses (the MUST-re-grep-before-landing instruction and does-NOT-depend-on-#804). The immutable DL-326 cell was already merge-order-safe (name+issue cite), so this only corrected mutable prose. - MEDIUM (unknown-count consistency): round-1 widened the ledger cell from five to six unknowns (adding runner-on-darwin / OQ-12) but left three "five" sites in the record body: the Why-STAGED enumeration, the reject-default alternative, and the T-1 task heading. Aligned all three to six + OQ-12, and aligned the threat classification (OQ-1, OQ-2, and OQ-12 threaten adoption) with the cell's widened re-open trigger. - LOW: re-wrapped two range-overwrite orphan lines to the file's ~72-col width. Checks: markdownlint 0 errors; design-ledger-gate OK (291 rows, 122 headers). Status stays Draft -- freezes Active on merge. Refs RIG-3238 Co-authored-by: Matt Wilkinson <matt@rigel.build>
Design addendum capturing Matt's ruling that the token-subject model admits a THIRD principal class, `SubjectService SubjectKind = 2` — one class for every first-party supervised compute tier that authenticates back to the Server (the RIG-1715 LLM gateway now, a future MCP gateway later), never a kind per tier. The current code seals `SubjectKind` "to exactly these two" (SubjectAccount=0, SubjectRunner=1), citing a since-retired v0.6 milestone record. Per the corpus convention (a frozen record is amended by a NEW record, never rewritten), this addendum is the amendment surface for that seal. Shipped BEFORE the implementation (RIG-3122 PR2) because the enum number and the `tokens.subject_kind` CHECK constraint land in `0001_init.sql` and are painful to rename once token rows exist. The record's Plan is the PR2 executor contract: the `SubjectService` const + seal-comment update, the CHECK `IN (0,1)`->`IN (0,1,2)` edit, the nolint text, the service-door mount via `ResolveToken(...want=SubjectService)`, and the 3x3 cross-door pgtest matrix. Isolation is per-Subject-ID + per-surface authz over the shared resolver's `want != Kind` gate — no new resolver, mirroring how SubjectAccount and SubjectRunner isolate today. Ledger: DL-325 (Topology & tiers). Ledger-impact: DL-325 (new) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…863) Review of #868 returned 0 high / 1 medium / 2 low. Citations (16/16) and the SubjectService ledger row were a clean pass; the blocker was executor-contract coherence. - F1 (medium): the Plan framed all five tasks as one "PR2 executor contract", but T4 (the service-door mount) targets a Server surface that does not exist in the tree yet, and no corpus task owns minting a SubjectService token — so the class as written ships unreachable. Reframed the Plan into two scopes: the enum half (T1/T2/T3/T5) lands now as PR2 (the urgent half — enum number + CHECK are painful to change once token rows exist); the door half (T4) lands WITH the RIG-2863 (RIG-1715 T2) service surface it mounts on, ordered after it. Added OQ-3 naming where token issuance lives (the RIG-2863 T4 slice, an IssueServiceToken mirroring MintRunnerToken) so an executor never improvises a mint path on a security-critical door. - F2 (low): citation span `lines 348-350` -> `348-351` (the quoted "disable)" falls on 351). - F3 (low, pre-existing): noted that `runnerhub.RunnerTokenRegistered` is the one token-store consumer outside the kind-gated door contract — flagged for the T4 executor to kind-check or document, out of scope for this record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked, and the sibling encryption PR (#859) took DL-325/DL-326. Renumbered this record's SubjectService row to the next free id, DL-327 (was DL-325), leaving main's DL-324 untouched. Additive review-fix commit. Ledger-impact: DL-327 (SubjectService, renumbered from DL-325) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…n (RIG-2863) Round-2 review verified all four round-1 fixes good (F1 two-scope Plan, F2 citation, DL-327 renumber, security lens a clean pass — the kind-gate isolates the new class for free) and found no high, three mediums + two lows. All are internal-consistency / constraint-accuracy defects (two are residue the F1 reframe left behind); all verified against the tree before folding; none is a design fork. - **M1 (slice-shorthand normalization):** the record used three shorthands for the RIG-2863 (= RIG-1715 T2) slice, one of which — "the RIG-2863 T4 slice" — collides with the real RIG-1715 T4 (compass-obs usage-metering), risking an executor routing IssueServiceToken to the wrong lane. Normalized all sites: Tasks T4 line now "(lands in RIG-2863 = RIG-1715 T2, NOT PR2)"; OQ-3 heading now "lands in the RIG-2863 slice (RIG-1715 T2), alongside this record's T4". - **M2 (registry deferral retargeted):** the Subject-ID registry bullet still called the registry "a PR2 implementation detail of the T4 surface's allowlist" after this record moved T4 out of PR2 — an executor holding the PR2 contract would build allowlist scaffolding into the enum-only PR. Retargeted: it lands WITH T4 in the RIG-2863 slice, PR2 does not touch it. - **M3 (0001 in-place-edit safe condition):** the constraint + the DL-327 row said the CHECK edit is safe "before data exists", but migrate() is version-keyed (skips any version in schema_migrations, store.go:157-176), not data-keyed — a DB that has applied v1 keeps the stale CHECK regardless of row count and boots clean past the refuse-to-serve guard. Restated the condition as disposable-env / version-keyed (wipe+re-migrate or new ALTER; pre-GA posture per RIG-3106 #830, RIG-2861 T1 #715) in both the record and DL-327. - **L1:** T5 now names both harnesses + build tags precisely (`go/internal/auth/token_test.go:134-154`, harness `harness_pgtest_test.go`, tag `pgtest && unix`; store round-trip in `tokens_test.go`, tag `pgtest`) — an auth case added without the `unix` tag silently never compiles. - **L2:** added a doc-refresh clause to T4 (which is where the third door wrap actually appears) naming the two-door prose the third door invalidates (token.go:98-99, :79-81, interceptor.go:140-141, runnerhub/auth.go:4-14). Ledger-impact: DL-327 (row's in-place-edit safe-condition restated) Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…n (RIG-2863) Round-3 adversarial review of PR #868 returned 0H/2M/3L; the two mediums are internal-consistency defects the round-2 M3 restatement left behind (not regressions — all five round-2 fixes verified good against the tree). Both verified against main `8fef2846` before folding. None is a design fork. Additive commit atop the bookmark. - **M1 (contradicting data-keyed framing):** the round-2 M3 fix corrected the in-place-edit safe-condition in the Global Constraints bullet and DL-327 to version-keyed, but left the original DATA-keyed framing ("painful … once token rows exist") standing at the two sites that carry the record's urgency rationale (:86-88 Problem/Intent, :120-122 inside the PR2 executor contract), where it directly contradicted the corrected bullet — and UNDERSTATED the urgency (any applied-v1 DB is already stale, not just one holding rows). Restated both in version-keyed terms ("painful … once any non-disposable database has applied v1, per Global Constraints"). - **M2 (incomplete T4 doc-refresh list):** the round-2 doc-refresh clause presented four sites as the complete set ("refresh THEM in the same slice") but two more two-door enumerations are equally invalidated by the third door and were omitted: `types.go:89` (the cross-door EXAMPLE clause — a separate sentence from the seal sentence T1 rewrites at :90-91, so no task owned it) and `network_door.go:229-230, :299-301` (the door-mount cross-rejection comments at the site the service door mounts on). Added both and reframed the list as "at least these six sites" so the completeness claim is honest. - **L3 (undefined "PR2"):** defined PR2 once at first use (:86) — "the enum-half implementation PR, T1/T2/T3/T5, throughout this record" — the central scope discriminator was used 10× and never defined. - **L4 (missing negative CHECK test):** added a T5 store round-trip asserting `PutTokenHash` with `Subject{Kind: SubjectKind(3)}` FAILS a constraint violation — the one case that catches a fat-fingered `IN (0,1,2,3)` or a dropped constraint (the admits-2 round-trip passes identically either way). - **L5 (llm-gateway path drift):** left as-is — the cited `server/` path is correct; the drift lives in the target record + tracker, not this diff (per reviewer, not this PR's defect to fix). markdownlint clean; design-ledger-gate OK (292 rows, 123 headers); no bare `RIG-2863 T<n>` shorthand remains; DL-327 already version-keyed (no ledger edit). Record 15703 B (<50 KB, DL-327 needs no anchor). Ledger-impact: DL-327 (text unchanged) Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…(RIG-2863) Round-3 review returned 0 high / 4 medium / 3 low. All six round-2 fixes held; the blockers were three new grounded defects in round-2's reconcile-execution- context paragraph (the WithSystemRole ruling itself is correct and stands) plus one ledger-recording judgment call Matt ruled on. - M1 (medium): the paragraph justified WithSystemRole by claiming FORCE RLS binds even the owner at boot. False — the owner connection is a superuser and a superuser bypasses even FORCE (0001_init.sql:848-851; rls_pgtest_test.go: 282-284). Restated on the true, superuser-independent reason: the reconcile takes the compass_app path, a non-owner non-BYPASSRLS role confined to the bootstrap tenant, so a row under another tenant is silently skipped without WithSystemRole. - M2 (medium): the prescribed two-tenant same-name test is unconstructible — `secrets.name` is a global PRIMARY KEY (0001_init.sql:396), so at most one row per configured name exists deployment-wide. Reframed the motivation and the test to the real case: a row declared under a NON-bootstrap tenant. - M3 (medium): corrected the "fifth entrypoint" miscount — the shipped tree arms WithSystemRole at THREE call sites (consumer.go:316, hub.go:753/814), and hub.go:814 (forge-notification-ack) is already a BYPASSRLS site DL-315's four names omit. Enumeration restated against the shipped set. - L1/L2/L3 (low): cite the arming code (armQueue/beginTenantTx) not just the WithSystemRole entrypoint; quote DL-315 verbatim; move the reconcile test into T0's Tests enumeration. - M4 -> OQ-5 (Matt-ruled: a new ledger row): the DL-315 BYPASSRLS allow-list widening lands as DL-326 (Refines DL-315, which stays Active), restating the true shipped entrypoint set incl. the previously-unledgered forge-notification- ack arm. Discoverability at the surface where the BYPASSRLS-audit question gets asked, over prose buried in an at-rest-encryption record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked. Renumbered this addendum's ledger rows to the next free sequential ids — the `gateway_credentials` encryption decision is DL-325 (was DL-324), the BYPASSRLS allow-list widening stays DL-326, and the sibling SubjectService PR (#868) takes DL-327 — leaving main's DL-324 untouched. Additive review-fix commit atop the round-2 fix. Ledger-impact: DL-325 (encryption, renumbered from DL-324), DL-326 (new, Refines DL-315) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-subject-service-design
branch
from
September 4, 2026 21:08
e3e30aa to
23f7473
Compare
…n (RIG-2863) Round-4 adversarial review returned 1H/3M/2L on the SubjectService design addendum. All round-3 fixes verified good; every new finding grounded against the tree at eb5ef7a before folding. Folded as a new commit atop the bookmark (additive review-fix convention). - H1 (service-door tenant posture — undesigned security fork handed to T4): the record fixed the auth PRINCIPAL but never its TENANT posture. A service door is a request path; under RLS a request-path statement arms compass_app + a tenant GUC (tenant_tx.go:135-143) and resolveTenant falls back to the bootstrap tenant (tenant.go:54-59), so a service door that sets no tenant runs bootstrap-scoped and sees one tenant's rows — silently breaking the record's first consumer, the LLM gateway stack-token surface, which must serve EVERY tenant's creds (llm-gateway design.md:333-337). The only cross-tenant escape, WithSystemRole, is explicitly fenced from request paths (tenant_tx.go:41-47). Added OQ-4 surfacing the fork with the three shapes T4 must choose among — (a) per-request tenant + store.WithTenant, (b) a Matt-ruled BYPASSRLS widening this record does NOT grant, (c) formal deferral to the RIG-2863 slice as a blocking T4 prerequisite — mirroring OQ-3's issuance deferral. Not decided here: it's a T4-time ruling for Matt at the design gate. Also corrected the account-door precedent to note account isolation is the tenant GUC layered with owner_user_id, not owner_user_id alone. PR2 (the enum half) does not depend on it. - M1 (network_door.go citation unresolvable): the round-3 fix fused a composite quote present at neither cited site and gave no resolvable path (bare network_door.go, which lives at go/server/, not go/internal/). Split into the two real sites with each site's actual text and the full go/server/ path. - M2 (types.go:89 under-range → false non-collision claim): the example clause spans :89-90 and SHARES line 90 with the seal sentence T1 rewrites, so the "do not collide" claim was false. Corrected the citation to :89-90 and made T1's edit sentence-scoped (rewrite the seal sentence in place, preserving the clause tail on line 90) rather than a destructive :90-91 line-range replace. - M3 (incomplete doc-refresh list): the "at least six sites" hedge left four+ more two-door enumerations unnamed, including one in the struct T1 edits and a proto comment stating a literal door COUNT. Replaced the fragile line-pinned list with a discovery RULE (a regex over go/ and proto/ comments that enumerate/count the door set) and kept the enumeration as evidence, now including types.go:86-88, types.go:101-103, mint.go:5-7, handler.go:68-69 / :260-261, and proto/compass/v1/runner.proto:53-56 / :183-184. - L1: specified the T5 negative-case assertion (assert err != nil; a 23514 falls through to a bare wrap, so do NOT add a store sentinel/SQLSTATE const — an unscoped API change). - L2: noted in Global Constraints that RunnerTokenRegistered (mint.go:80-93) is a kind-agnostic store-level existence check outside the one-resolver invariant, whose false-"registered" surface the third class widens by one. markdownlint clean; design-ledger-gate OK (292 rows, 123 headers valid; record 21740 B < 50 KB, DL-327 needs no anchor). Ledger-impact: none (DL-327 unchanged). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…n (RIG-2863)
Round-5 review of the SubjectService design addendum returned 0 high / 2
medium / 3 low. All six round-4 fixes verified holding against the tree at
`eb5ef7a1`; the two mediums are defects the round-4 M3 rewrite (the T4
doc-refresh discovery rule) introduced in itself. All driver-foldable, no
design fork.
- M1 (discovery-rule regex missed its own first evidence site): the rule's
regex matched only `account (token|subject)` / `Runner (token|subject)`, but
its first cited site `token.go:98-99` reads "account **door**" / "Runner
**door**" (and `:71`/`:94` "the other **door**"). A T4 executor running the
rule — which the record elevates to THE contract over the line-pinned list —
would miss the single most load-bearing door enumeration in the codebase.
Added a `door` alternative and `(the )?other door`.
- M2 (rule missed the door-COUNT axis entirely): scoped to the subject-KIND
split, the rule slid past `doc.go:7` ("Two doors reach the same compass.v1
service"), `interceptor.go:86`/`:129`, and `service.go:539`/`:604` ("one on
both doors") — all door-count enumerations that go stale when a third door
mounts. Widened the regex with `both doors|two doors`, added `doc.go:7-19`
(package-level count), `interceptor.go:86`/`:129`, `service.go:539`/`:604`
to the evidence list, and named the two enumeration axes (kind vs
socket-vs-network door count) explicitly so an executor does not assume
kind-keyed matching is exhaustive.
- L1 (types.go:101-103 mis-scheduled to T4): the `Subject` struct doc's
two-kind enumeration is a KIND-axis enumeration that goes stale the moment
T1 lands the third kind — leaving a "the account or Runner" struct doc
directly above an `ID` field doc T1 has just extended to a third id space.
Moved it from the T4 door-refresh list into T1 (same comment block T1
already edits).
- L2 (runner.proto:183-184 mis-attributed): the literal-COUNT rationale
belongs to `:53-56` alone; `:183-184` ("account-subject tokens rejected")
is an ordinary stale two-kind enumeration, not a count. Split the
justification.
- L3 (T2 CI consequences unnamed): a frozen executor contract naming the
exact `0001_init.sql` line to edit should name the gates that edit trips.
Added to T2: after the CHECK widen, run `moon run compass-go:sqlc-gen` and
confirm no drift (the `sqlc-drift` gate fails closed on any stale
`internal/store/db` byte), and expect `sql-migration-gate` (squawk + sqruff
over `go/internal/store/migrations/*.sql`) to re-run over the edited file.
Additive review-fix commit atop the round-4 fold (record-only; DECISIONS.md
untouched). markdownlint 0 errors; design-ledger-gate OK (292 rows, 123
headers; record 23313 B < 50 KB so DL-327 carries no anchor). All findings
verified against the tree before folding.
Refs RIG-2863
Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-server/rig-2863-subject-service-design
branch
from
September 4, 2026 22:51
3499814 to
65ffcb8
Compare
…n (RIG-2863) Round-6 review returned 0 high / 2 medium / 3 low on PR #868; all five grounded against the tree at `eb5ef7a1` and folded additively. Both mediums were defects the round-5 T4 discovery-rule rewrite introduced in itself. ### Mediums - **M2 — the "two doors after PR2" premise is FALSE at the tree.** The record claimed "there are still only two DOORS after PR2, so the two-door enumerations stay literally true until T4". But a third Kind-gated `compass.v1` door already exists at `eb5ef7a1`: RunnerService, mounted at `network_door.go:313` behind `runnerhub/auth.go:79`'s `SubjectRunner` bearer; `serve.go:572`/`:647`/`:662` count "three compass.v1 doors". Corrected the premise and re-scoped: this record changes the subject-KIND axis, not the door count. The `auth`-package "two doors" sentences (`doc.go:7`, `interceptor.go:86`/`:129`, `service.go:539`/`:604`) are scoped to that package's OWN network+socket `CallerFrom` pair (`doc.go:2` scopes the package to "the network door"; RunnerService authenticates in `runnerhub`, not `auth`) — narrow-and-true within scope, and OUT of scope for both PR2 and T4. Restructured the evidence list into kind-axis refresh targets vs the out-of-scope door-count sites. - **M1 — discovery regex case-sensitive against its own new evidence site.** The round-5 widened regex carried lowercase `two doors`, but `doc.go:7` opens capital-T "Two doors" at sentence start, so the count sentence the rule was widened to catch was invisible to it (matched only incidentally via `:18`'s "both doors"). Since the door-count axis is now out of scope (M2), the rule is scoped to the kind axis and stated case-INSENSITIVE. ### Lows - **L1** — a `runner.proto` comment edit is mirrored into the checked-in, non-gitignored generated tree (`runner.connect.go:98`/`:367`), drift-gated by `compass-proto:drift` (in `proto:ci`); noted regenerate-and-commit, never hand-edit. - **L2** — named `RunnerTokenRegistered`'s two callers: the operator CLI (`compass-mint-runner-token/main.go:157`) and the automated stack-boot heal (`stack/adapters/token.go:87`); clarified neither is an untrusted-input path. - **L3** — scoped the discovery rule to NON-GENERATED comments (exclude `go/gen/**`, `go/internal/gen/**`), with test prose in scope. Design record only; no code or proto change. OQ-4 (round-4 HIGH, service-door tenant posture) re-verified byte-identical and undisturbed — still surfaces-not-decides the three-shape fork. Security lens re-confirmed: the additive `SubjectService = 2` enum is inert at every door (sole resolver `auth.ResolveToken`; both doors pass explicit `want`; no `SubjectKind` switch; no proto mirror). markdownlint 0, design-ledger-gate OK (292 rows, record 25130 B < 50KB so DL-327 carries no anchor). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…n (RIG-2863)
Round-7 review returned 0H/2M/3L; both mediums were self-inflicted
regressions from the round-6 fold (the recurring defect-while-fixing
pattern). All five folded, each grounded against the tree at `eb5ef7a1`.
- M2 (regression): the round-6 out-of-scope rationale claimed the
surviving "two doors" count sentences are scoped to the `auth`
package's OWN network+socket `CallerFrom` pair. False at the tree —
THREE `compass.v1` doors mount the auth ambient/`CallerFrom` pair
(socket serve.go:708-710, dev loopback serve.go:745-747, network
network_door.go:279-283), all read via CallerFrom (interceptor.go:44),
and it contradicted the record's own three-doors sentence two lines
up. Replaced the false package-scoping with the sound axis argument
alone (door-count is orthogonal to subject-kind), plus a neutral note
that `auth/doc.go:7`'s literal count is a pre-existing doc question
this record neither creates nor owns.
- M1 (regression): the amended discovery regex, scoped to go/ and proto/
with .sql in scope by its own wording, structurally could not reach
0001_init.sql:372-373 ("subject_kind is 0 account / 1 runner") — a
real two-kind gloss T2 already mandates editing. Extended the regex
with `0 account|1 runner`, stated .sql migration comments explicitly
in scope, broadened the exclusion to the checked-in sqlc tree
(go/internal/store/db/**), and named 0001_init.sql:372-373 in the
evidence list (cross-referencing that T2 owns the edit).
- L1: bare `token.go:87` shorthand collided with the record's own
convention (bare token.go = go/internal/auth/token.go, a real
unrelated line); disambiguated to stack/adapters/token.go:87.
- L2: `proto:ci` corrected to `compass-proto:ci` (.moon/workspace.yml:18
maps compass-proto -> proto; the CI target uses the project id).
- L3: re-wrapped the round-6 range-edit's over-long prose lines to the
record's ~81-char norm.
Record 25130 -> 25804 B (<50KB, DL-327 correctly anchor-free).
markdownlint 0, design-ledger-gate OK (292 rows, 123 headers). Amended
M2 regex verified to now match 0001_init.sql:373 against the tree.
Refs RIG-2863
Spec-impact: design record only; no code or ledger-row change.
Co-authored-by: Matt Wilkinson <matt@rigel.build>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design addendum capturing Matt's ruling that the token-subject model admits a
THIRD principal class,
SubjectService SubjectKind = 2— one class for everyfirst-party supervised compute tier that authenticates back to the Server (the
RIG-1715 LLM gateway now, a future MCP gateway later), never a kind per tier.
The current code seals
SubjectKind"to exactly these two" (SubjectAccount=0,SubjectRunner=1), citing a since-retired v0.6 milestone record. Per the corpus
convention (a frozen record is amended by a NEW record, never rewritten), this
addendum is the amendment surface for that seal.
Shipped BEFORE the implementation (RIG-3122 PR2) because the enum number and the
tokens.subject_kindCHECK constraint land in0001_init.sqland are painfulto rename once token rows exist. The record's Plan is the PR2 executor contract:
the
SubjectServiceconst + seal-comment update, the CHECKIN (0,1)->IN (0,1,2)edit, the nolint text, the service-door mount viaResolveToken(...want=SubjectService), and the 3x3 cross-door pgtest matrix.Isolation is per-Subject-ID + per-surface authz over the shared resolver's
want != Kindgate — no new resolver, mirroring how SubjectAccount andSubjectRunner isolate today.
Ledger: DL-325 (Topology & tiers).
Ledger-impact: DL-325 (new)
Spec-impact: none
Refs RIG-2863
Co-authored-by: Matt Wilkinson matt@rigel.build